Unrendered is not one thing: split prose from structure and control (#104) - #482
Open
jeremymanning wants to merge 2 commits into
Open
Unrendered is not one thing: split prose from structure and control (#104)#482jeremymanning wants to merge 2 commits into
jeremymanning wants to merge 2 commits into
Conversation
`orchestrator validate --json` has emitted structured findings since #467. `PipelineAPI.validate_yaml` returns a bare `bool`, so a caller embedding the orchestrator could learn *that* a document was rejected and nothing about why, and could not see warnings at all -- which is where "this reference could not be checked" lives, the warning that precedes the run-time failure in #465. The findings were not missing, only private to `cli.py`. Exposing them by writing a second implementation would have created two things to drift apart, which is the bug #466 removed for dependencies, so `validation/pipeline_report` is the one implementation and the CLI formats what it returns. A test compares the CLI's JSON against the API's objects to keep it that way. While testing it, `validate` turned out not to be reproducible. Three runs of the same command over the same file produced the same 44 findings in three different orders, from two hash-order sources: * findings were emitted while iterating `set(var_names) | set(...)`; * `_suggest_similar_names` iterated an unsorted candidate list *and truncates to three*, so on a longer list it would have offered different suggestions run to run rather than merely reordering them. Both are now sorted, and repeated runs are byte-identical. The in-process check cannot catch this -- `PYTHONHASHSEED` is fixed for the life of a process -- so the test runs three subprocesses. The first version of this suite was written against `examples/supported/01_hello_filesystem.yaml`, which produces no findings at all: deleting every warning from the implementation still passed all of it. It is now anchored on a document carrying 44 findings, with explicit guards so an empty list cannot pass as agreement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…104) #480 applied `core/step_fields` to template validation and got the first half right -- a stray brace in a `description:` no longer rejects a pipeline that runs. It got the second half wrong by treating *every* unrendered field as prose, so all of these compiled with nothing but a wording note: metadata: goto: "{{ nosuch }}" # execution sent to a step named `{{ nosuch }}` priority: "{{ nosuch }}" # a priority of that text requires_model: "{{ nosuch }}" tool: "{{ nosuch }}" # a registry lookup for that text id: "{{ nosuch }}" Unrendered says what does *not* happen to a field, not what the field is for. Three cases, not one: * **prose** -- `name`, `description`, and metadata an author wrote for themselves. Nothing acts on it; the braces reach a log line and the pipeline runs. Still a warning, because the author will not get what they typed. * **structural** -- `id`, `tool`, `dependencies`, `depends_on`. These *name* things. A literal `{{ x }}` names nothing, so the pipeline is already broken and calling it valid says the opposite. Now an error. * **operational metadata** -- the keys the runtime reads. Not rendered, so control code receives the literal template text. Now an error. `OPERATIONAL_METADATA_KEYS` is evidence-backed rather than guessed: every key has a runtime read cited beside it, from `goto` at `orchestrator.py:1074` to `required_capabilities` at `core/control_system.py:130`. Keys the compiler *writes* -- `step_type`, `retry_count`, the loop bookkeeping -- are excluded, because an author never supplies them. Two boundaries the split needs beyond the three sets. A reserved key counts only at metadata's top level: `metadata.notes.priority` is somebody's data structure, not the key the runtime reads. And step recognition is now off entirely inside an inert subtree -- `metadata.steps` holding `for_each` and `while` was being walked as pipeline structure and reported as an ambiguous loop, from inside a subtree this module had just declared verbatim-copied. Separately, `create_template_issue` hardcoded `code="template_error"`, so every template finding arrived under one code and a consumer had to parse the message to tell an inert-field note from a loop-scope error. The validator's own `error_type` now flows through the compiler into the finding, which is what the structured payload exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corrects the finding against #480. Merge after #481 — this is stacked on it.
The defect
#480 got the first half right: a stray brace in a
description:no longer rejects a pipeline that runs. It got the second half wrong by treating every unrendered field as prose. Reproduced against mergedmain:So a
gotosending execution to a step literally named{{ nosuch }}was reported as a wording problem, and the pipeline as valid."Unrendered" says what does not happen to a field. It does not say what the field is for.
Three classes
name,description, arbitrarymetadataid,tool,dependencies,depends_on{{ x }}names nothingOPERATIONAL_METADATA_KEYSis evidence-backed rather than guessed. Each key has a runtime read cited beside it in the docstring:Keys the compiler writes —
step_type,retry_count, the loop bookkeeping — are deliberately absent: an author never supplies them, so a template in one is not a case that arises.Two boundaries the sketch does not cover
A reserved key counts only at metadata's top level.
metadata.notes.priorityis somebody's data structure, not the key the runtime reads, and erroring on it would be a new false rejection.Step recognition is now off entirely inside an inert subtree, rather than special-casing
steps. Yourmetadata.stepscase was the symptom; the cause is that traversal was doing structural recognition inside a subtree this module had just declared verbatim-copied. Special-casing the one key would leave the next one to be found.The diagnostic code
create_template_issuehardcodedcode="template_error", so every template finding arrived under one code and a consumer had to parse the human message to tell an inert-field note from a loop-scope error. The validator's ownerror_typenow flows through the compiler into the finding — which is what the structured payload exists for, and it also makes the error-code dashboard (items 9–10) feasible.Verification
ruff check src/orchestrator --select E9,F63,F7,F82,F821,F823,F601,F811— cleanGates were run in an isolated
git worktree, so a branch switch in the shared checkout cannot mix trees mid-run.Not merging
#479 and #480 both went in without independent review, and this is a correction to the second of them.